-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Frontend][Review Metrics] Show old values #36
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@terryttsai can you upload a video or screenshot?! Excited to see this!
@@ -280,11 +281,27 @@ const ReviewMetrics: React.FC = observer(() => { | |||
value: string | number | null, | |||
oldValue: string | number | null | |||
) => { | |||
// Uncomment this for special treatment when a datapoint is newly added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thanks for putting the foundation here. I wonder if we should uncomment this for now, to facilitate debugging and also just to see how it feels? Maybe we can change the color or something instead of saying NEW?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what the new symbol should be, it could also be less noisy to instead have an indication when the value has been unchanged. I'll leave that up to a follow-up PR
// } | ||
if (oldValue !== value) { | ||
return ( | ||
<DatapointsTableDetailsCell key={key}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this!
Added a screenshot |
Terry - this looks AMAZING!!! |
I know this is such a cool change!! What a great idea to do this 😍 |
Description of the change
Show old values in the Review Metrics page
Optional code to show when a metric is "new", as in the previous value was Null
Type of change
Related issues
Closes #XXXX
Checklists
Development
This box MUST be checked by the submitter prior to merging:
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: